Skip to content

gh-104219: Document that idunders can return NotImplemented#104220

Merged
erlend-aasland merged 6 commits into
python:mainfrom
Gouvernathor:patch-4
Mar 1, 2024
Merged

gh-104219: Document that idunders can return NotImplemented#104220
erlend-aasland merged 6 commits into
python:mainfrom
Gouvernathor:patch-4

Conversation

@Gouvernathor

@Gouvernathor Gouvernathor commented May 6, 2023 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

I tried to change as little as possible but a broader reformulation, or a separate paragraph explaining that when __iadd__ returns NotImplemented it's the same as if iadd didn't exist at all, could be deemed more readable.

📚 Documentation preview 📚: https://cpython-previews--104220.org.readthedocs.build/

@bedevere-bot bedevere-bot mentioned this pull request May 6, 2023
Closed
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir skip news awaiting review labels May 6, 2023
CAM-Gerlach
CAM-Gerlach reviewed May 6, 2023
View reviewed changes

@CAM-Gerlach CAM-Gerlach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using the relevant role so that NotImplemented is appropriately formatted and readers can easily jump to more information.

Additionally, you might want to add the role to the adjacent, otherwise-similar descriptions of the __add__ and __radd__ dunders as well (and perhaps explicitly mention for __add__ that the operation falls back to __radd__ if NotImplemented is returned?).

Comment thread Doc/reference/datamodel.rst Outdated
Comment thread Doc/reference/datamodel.rst Outdated
@CAM-Gerlach CAM-Gerlach added the needs backport to 3.11 only security fixes label May 6, 2023
Gouvernathor and others added 3 commits May 6, 2023 14:34
@Gouvernathor @CAM-Gerlach
Markup and reference NotImplemented
f6c73f6 
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@Gouvernathor @CAM-Gerlach
Markup and reference NotImplemented too
89bb464 
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
@Gouvernathor
Use :const: for other uses of NotImplemented
a317522
@Gouvernathor

Gouvernathor commented May 6, 2023

Copy link
Copy Markdown
Contributor Author

I did that, and in other places too (not in the NotImplemented section itself and only once in some paragraph where it comes up 3 or 4 times).
I think the __add__ explanation is clear enough as it is, since more explanations are given just afterwards, and even then I think it's not really the purpose of this PR.

erlend-aasland
erlend-aasland reviewed Feb 29, 2024
View reviewed changes
Comment thread Doc/reference/datamodel.rst Outdated
@erlend-aasland erlend-aasland added the needs backport to 3.12 only security fixes label Feb 29, 2024
JelleZijlstra
JelleZijlstra reviewed Feb 29, 2024
View reviewed changes
Comment thread Doc/reference/datamodel.rst Outdated
@Gouvernathor @JelleZijlstra
Rephrase and clarify
1dd9905 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@erlend-aasland erlend-aasland merged commit 2713c2a into python:main Mar 1, 2024
@miss-islington-app

This comment was marked as outdated.

Sign in to view
@erlend-aasland

erlend-aasland commented Mar 1, 2024

Copy link
Copy Markdown
Contributor

Thanks for the report and PR!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 1, 2024
@Gouvernathor @CAM-Gerlach
@JelleZijlstra @miss-islington
pythongh-104219: Document that idunders can return NotImplemented (py…
d22d97a 
…thonGH-104220)

(cherry picked from commit 2713c2a)

Co-authored-by: Gouvernathor <44340603+Gouvernathor@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app

bedevere-app Bot commented Mar 1, 2024

Copy link
Copy Markdown

GH-116210 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 1, 2024
@Gouvernathor @CAM-Gerlach
@JelleZijlstra @miss-islington
pythongh-104219: Document that idunders can return NotImplemented (py…
b1706a2 
…thonGH-104220)

(cherry picked from commit 2713c2a)

Co-authored-by: Gouvernathor <44340603+Gouvernathor@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Mar 1, 2024
@bedevere-app

bedevere-app Bot commented Mar 1, 2024

Copy link
Copy Markdown

GH-116211 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Mar 1, 2024
@Gouvernathor Gouvernathor deleted the patch-4 branch March 1, 2024 15:59
@Gouvernathor

Gouvernathor commented Mar 1, 2024

Copy link
Copy Markdown
Contributor Author

Thank you !

erlend-aasland pushed a commit that referenced this pull request Mar 1, 2024
@miss-islington @Gouvernathor
@CAM-Gerlach @JelleZijlstra
[3.12] gh-104219: Document that idunders can return NotImplemented (G…
694874b 
…H-104220) (#116210)

(cherry picked from commit 2713c2a)

Co-authored-by: Gouvernathor <44340603+Gouvernathor@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
erlend-aasland pushed a commit that referenced this pull request Mar 1, 2024
@miss-islington @Gouvernathor
@CAM-Gerlach @JelleZijlstra
[3.11] gh-104219: Document that idunders can return NotImplemented (G…
9eb7ed7 
…H-104220) (#116211)

(cherry picked from commit 2713c2a)

Co-authored-by: Gouvernathor <44340603+Gouvernathor@users.noreply.github.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
@Gouvernathor @CAM-Gerlach
@JelleZijlstra @woodruffw
pythongh-104219: Document that idunders can return NotImplemented (py…
9cd4396 
…thon#104220)

Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@Gouvernathor @CAM-Gerlach
@JelleZijlstra @adorilson
pythongh-104219: Document that idunders can return NotImplemented (py…
f4730a7 
…thon#104220)

Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@Gouvernathor @CAM-Gerlach
@JelleZijlstra @diegorusso
pythongh-104219: Document that idunders can return NotImplemented (py…
74731d2 
…thon#104220)

Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@CAM-Gerlach CAM-Gerlach Awaiting requested review from CAM-Gerlach

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Gouvernathor @erlend-aasland @JelleZijlstra @CAM-Gerlach @bedevere-bot